-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[Proxy] feat: implement relayer proxy full workflow #101
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just left one update in a TODO comment, but otherwise really clean and g2g!
Co-authored-by: Daniel Olshansky <[email protected]>
@bryanchriswhite After you merge in #65, let's try to rebase & update this PR as it should be g2g. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@red-0ne Please merge (and fix conflicts) with main, and will re-review then.
Please also add *_test.go
files with a TODO_TEST
comment where applicable explaining that we're biasing towards an E2E relay
and that building test utilities for these off-chain components is a non-trivial amount of work.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reviewed everything with the exception of pkg/relayer/proxy/relay_verifier.go
. Going to check that out tomorrow. Great work!
This PR should be merged after #150 |
@h5law , we switched from only signing the |
pkg/relayer/interface.go
Outdated
|
||
// SignRelayResponse is a shared method used by RelayServers to sign the relay response. | ||
SignRelayResponse(relayResponse *types.RelayResponse) ([]byte, error) | ||
// This method may mutate the relay request in the future, so the returned |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's kind of confusing that:
- We pass in a pointer to an object
- That pointer may be mutated
- We return it
I believe it should be one of:
- Pass a pointer that may be mutated
- Pass a value that's copied, mutated, and returned
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see... I'll revert to passing the pointer without returning and mutate it inside the function as I can't clearly foresee the impact of changing the whole workflow to not use pointers. We'll keep that question to when we start implementing the middleware capabilities.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@red-0ne Also, please update this PR w/ main |
* feat: add general proxy logic and server builder * fix: make var names and comments more consistent * feat: add relay verification and signature * fix: interface assignment * feat: implement relayer proxy full workflow * chore: improve code comments * chore: change native terms to proxied * chore: address change requests * chore: explain -32000 error code * chore: add log message on relaying success/failure * chore: Update AccountI/any unmarshaling comment Co-authored-by: Daniel Olshansky <[email protected]> * chore: update import paths to use GitHub organization name * chore: address change requests * chore: add TODO for test implementation * chore: Refactor relay request & response methods in RelayerProxy * chore: Address request changes * chore: comment about current proxy signing approach * Merge remote-tracking branch 'origin/main' into feat/relayer-proxy * chore: Revert relay request & response signing and verification interface --------- Co-authored-by: Daniel Olshansky <[email protected]>
Summary
Human Summary
Implemented the needed parts of the
RelayerProxy
flow which involves:RelayRequest
from thehttp.Request
RelayRequest
signature and sessionRelayReponse
and signing itIt assumes the availability
BlockClient
andSession
query client with the appropriateSessionHeader
fields (available when PRs #78 and #65 get merged)AI Summary
Summary generated by Reviewpad on 08 Nov 23 22:31 UTC
This pull request includes various changes across multiple files in the codebase. Here's a summary of the changes:
In the file "file_diff.go":
In the file "relay_signer.go":
In the file "interface.go":
In the file "error_reply.go":
In the file "other_file_diff.go":
In the file "interface.go":
In the file "proxy_test.go":
In the file "relay.proto":
In the file "jsonrpc.go":
In the file "relay_verifier.go":
In the file "errors.go":
In the file "new_file.go":
Please let me know if you need more information.
Issue
Type of change
Select one or more:
Testing
make go_test
Sanity Checklist